-
Notifications
You must be signed in to change notification settings - Fork 545
Propagate tool call exceptions through filters #844
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
FIne by me -- Just need a way to handle. My plan was to dump ex.Message in most cases. Thank you! |
McpProtocolException derives from McpException which can still be used to control the JsonRpcErrorDetail set by McpSessionHandler and the CallToolResult error content set by McpServerImpl making this change non-breaking unless you were previously setting an ErrorCode
Co-authored-by: Stephen Toub <[email protected]>
Hi, |
It's in 0.4.0-preview.3 |
I thought I added tests that verified that tool call exceptions propagated through filters before getting converted to a CallToolResult with
IsError = true
when I originally added the filter pipeline in #733, but I did not. If I had, I would have noticed that I put the exception catching logic immediately around the call to McpServerTool.InvokeAsync or the custom CallToolHandler without letting it propagate through the entire filter pipeline which was not my intention.This PR adds a new Exception type, McpServerToolException, because authorization filters still need the ability to throw an McpException to produce a JsonRpcError like it does in other handler types. McpServerToolException allows McpServerTool implementations to retain the ability to produce an CallToolResult with
IsError = true
and a custom message with an exception like before. However, I think even if not for the authorization filters, this change makes sense considering that McpException is "not intended to be used for application-level errors.", and it includes details irrelevant to the CallToolResult like the McpErrorCode.csharp-sdk/src/ModelContextProtocol.Core/McpException.cs
Line 8 in f286391
Technically, it isn't strictly necessary for McpServerToolException to exist considering any McpServerTool or CallToolHandler can just return a CallToolResult with
IsError = true
, but that might be inconvenient.Fixes #820
This doesn't fix #830, since ArgumentException's from ReflectionAIFunctionDescriptor.GetParameterMarshaller will still get turned into generic errors without further intervention. But least after this change, developers will be able to add a tool call filter that observes the ArgumentException and turns it into a McpServerToolException (to produce a CallToolResult with IsError = true) or McpException (to produce a JsonRpcError with an McpErrorCode) depending on their needs.